Add suspenseCallback feature for runtime tracing of loading states by bgirard · Pull Request #16134 · facebook/react

bgirard · 2019-07-15T06:22:23Z

This adds a 'SuspenseCallback' feature flag. When the property is set on
a suspense component it will be called during the commit phase with a
set of the immediate thenable for this component. This will allow user
code to build runtime tracing of the cause for a suspense boundary.

CC @sebmck

sizebot · 2019-07-15T07:34:02Z

ReactDOM: size: 0.0%, gzip: 0.0%

Details of bundled changes.

Comparing: 19354db...448fa2c

react-art

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-art.development.js	+0.1%	+0.1%	639.48 KB	640.32 KB	139.2 KB	139.38 KB	UMD_DEV
react-art.development.js	+0.1%	+0.2%	570.36 KB	571.2 KB	121.85 KB	122.03 KB	NODE_DEV
ReactART-dev.js	+0.1%	+0.1%	584.13 KB	584.8 KB	121.36 KB	121.51 KB	FB_WWW_DEV
ReactART-prod.js	🔺+0.3%	🔺+0.3%	219.04 KB	219.67 KB	37.2 KB	37.3 KB	FB_WWW_PROD

react-dom

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-dom-server.browser.development.js	+0.1%	+0.2%	137.32 KB	137.51 KB	36.07 KB	36.16 KB	UMD_DEV
ReactDOM-dev.js	+0.1%	+0.1%	928.61 KB	929.28 KB	204.98 KB	205.11 KB	FB_WWW_DEV
react-dom-server.browser.production.min.js	0.0%	0.0%	19.41 KB	19.41 KB	7.26 KB	7.26 KB	UMD_PROD
react-dom-test-utils.development.js	+0.3%	+0.5%	58 KB	58.19 KB	15.92 KB	15.99 KB	UMD_DEV
react-dom-unstable-fizz.browser.development.js	0.0%	-0.1%	3.78 KB	3.78 KB	1.52 KB	1.52 KB	UMD_DEV
react-dom-test-utils.production.min.js	0.0%	0.0%	10.95 KB	10.95 KB	4.01 KB	4.01 KB	UMD_PROD
react-dom-unstable-fizz.browser.production.min.js	0.0%	🔺+0.1%	1.21 KB	1.21 KB	706 B	707 B	UMD_PROD
react-dom-test-utils.development.js	+0.3%	+0.5%	56.26 KB	56.45 KB	15.6 KB	15.67 KB	NODE_DEV
react-dom-unstable-fizz.browser.development.js	0.0%	-0.1%	3.61 KB	3.61 KB	1.48 KB	1.48 KB	NODE_DEV
react-dom-test-utils.production.min.js	0.0%	🔺+0.1%	10.71 KB	10.71 KB	3.94 KB	3.95 KB	NODE_PROD
react-dom.development.js	+0.1%	+0.1%	905.2 KB	906.04 KB	204.61 KB	204.79 KB	UMD_DEV
react-dom.production.min.js	0.0%	0.0%	111.64 KB	111.64 KB	35.91 KB	35.92 KB	UMD_PROD
react-dom.development.js	+0.1%	+0.1%	899.49 KB	900.33 KB	203.11 KB	203.29 KB	NODE_DEV
react-dom-server.node.development.js	+0.1%	+0.2%	135.38 KB	135.57 KB	35.68 KB	35.77 KB	NODE_DEV
ReactDOM-prod.js	🔺+0.1%	🔺+0.2%	372.21 KB	372.73 KB	68.19 KB	68.3 KB	FB_WWW_PROD
ReactDOM-profiling.js	+0.1%	+0.1%	378.37 KB	378.89 KB	69.53 KB	69.61 KB	FB_WWW_PROFILING
react-dom-server.browser.development.js	+0.1%	+0.2%	133.45 KB	133.64 KB	35.15 KB	35.23 KB	NODE_DEV
react-dom-unstable-native-dependencies.development.js	0.0%	0.0%	60.76 KB	60.76 KB	15.85 KB	15.85 KB	UMD_DEV
ReactDOMServer-prod.js	0.0%	0.0%	46.83 KB	46.83 KB	10.77 KB	10.78 KB	FB_WWW_PROD
react-dom-unstable-fizz.node.development.js	0.0%	-0.1%	3.85 KB	3.85 KB	1.5 KB	1.5 KB	NODE_DEV
react-dom-unstable-fizz.node.production.min.js	0.0%	-0.1%	1.1 KB	1.1 KB	668 B	667 B	NODE_PROD

react-test-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
ReactTestRenderer-dev.js	+0.1%	+0.1%	595.17 KB	595.84 KB	123.87 KB	124 KB	FB_WWW_DEV
react-test-renderer-shallow.development.js	+0.5%	+0.8%	41.5 KB	41.69 KB	10.8 KB	10.88 KB	UMD_DEV
react-test-renderer-shallow.development.js	+0.5%	+0.9%	35.64 KB	35.83 KB	9.43 KB	9.51 KB	NODE_DEV
react-test-renderer.development.js	+0.1%	+0.1%	583.83 KB	584.47 KB	124.65 KB	124.75 KB	UMD_DEV
react-test-renderer.development.js	+0.1%	+0.1%	579.37 KB	580.01 KB	123.54 KB	123.64 KB	NODE_DEV

react-reconciler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-reconciler.development.js	+0.1%	+0.2%	568.52 KB	569.36 KB	120.38 KB	120.56 KB	NODE_DEV
react-reconciler.production.min.js	0.0%	0.0%	68.85 KB	68.85 KB	20.5 KB	20.5 KB	NODE_PROD
react-reconciler-reflection.development.js	+1.0%	+1.3%	18.78 KB	18.96 KB	6.06 KB	6.14 KB	NODE_DEV
react-reconciler-persistent.development.js	+0.1%	+0.1%	566.07 KB	566.72 KB	119.32 KB	119.41 KB	NODE_DEV

react-native-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
ReactFabric-profiling.js	0.0%	-0.0%	275.14 KB	275.14 KB	47.35 KB	47.34 KB	RN_OSS_PROFILING
ReactFabric-dev.js	+0.1%	+0.1%	731.89 KB	732.56 KB	154.3 KB	154.42 KB	RN_FB_DEV
ReactFabric-prod.js	0.0%	-0.0%	266.36 KB	266.36 KB	45.87 KB	45.87 KB	RN_FB_PROD
ReactNativeRenderer-dev.js	+0.1%	+0.1%	718.25 KB	718.92 KB	151.71 KB	151.83 KB	RN_OSS_DEV
ReactNativeRenderer-dev.js	+0.1%	+0.1%	718.35 KB	719.02 KB	151.76 KB	151.88 KB	RN_FB_DEV
ReactFabric-dev.js	+0.1%	+0.1%	731.79 KB	732.46 KB	154.26 KB	154.37 KB	RN_OSS_DEV

Generated by 🚫 dangerJS

sebmarkbage · 2019-07-15T18:24:15Z

packages/react-reconciler/src/ReactFiberCommitWork.js

+
+  if (
+    enableSuspenseCallback &&
+    'suspenseCallback' in finishedWork.memoizedProps


You can skip this check since it will be implied by the get below.

sebmarkbage · 2019-07-16T22:01:25Z

It's not obvious to me why this would break in the persistent renderer (used for RN Fabric). You may have stumbled upon an unrelated bug.

bgirard · 2019-07-17T01:45:25Z

The failure is caused by this line:

react/packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines 837 to 845 in 9f39590

    
           if (supportsPersistence) { 
        
             // TODO: Only schedule updates if not prevDidTimeout. 
        
             if (nextDidTimeout) { 
        
               // If this boundary just timed out, schedule an effect to attach a 
        
               // retry listener to the proimse. This flag is also used to hide the 
        
               // primary children. 
        
               workInProgress.effectTag |= Update; 
        
             } 
        
           }

My vague understanding is that I'm relying on this line to sometimes cause an update incidentally. I need to decide when an update needs to actually happen and trigger it directly.

I can fix it by forcing workInProgress.effectTag |= Update; always. Are we expecting completeWork to only cause an update (commit?) only when needed? That's what I'm exploring right now. I think I want to compare the current/WIP fiber and cause an update if their updateQueue (thenablesSet) changed.

Does that sound right?

packages/react-reconciler/src/ReactFiberCompleteWork.js

packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js

sebmarkbage · 2019-07-17T17:53:50Z

Unrelated but I found a bug in the "train model". Because we aggressively mark this for updates, we commit too often which marks the last commit time and pushes out the train model's next commit further than it should.

https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberCommitWork.js#L1266

sebmarkbage · 2019-07-17T18:02:18Z

I think there's a disconnect somewhere.

Why do you need the callback to fire after everything has been shown? It doesn't have any promises to give you since it's not waiting on anything. Wouldn't it be fine to just ignore the callback in this case?

packages/react-reconciler/src/ReactFiberCommitWork.js

bgirard · 2019-07-18T02:23:16Z

Applied the suggestions, thanks for the tip on fleshing out the callback behavior. I updated the user code to match with this and I think it looks decent. I'm getting great traces from this.

bgirard · 2019-07-19T23:48:29Z

rebased, should be ready to go.

packages/react-reconciler/src/ReactFiberCompleteWork.js

This adds a 'SuspenseCallback' feature flag. When the property is set on a suspense component it will be called during the commit phase with a set of the immediate thenable for this component. This will allow user code to build runtime tracing of the cause for a suspense boundary.

bgirard · 2019-07-23T22:10:25Z

The build is stuck, but I rebased and retest against FB WWW.

FokkeZB · 2022-12-13T13:02:11Z

@bgirard do you have any examples of how people use this flag and <Suspense> callback property?

facebook-github-bot added the CLA Signed label Jul 15, 2019

bgirard force-pushed the master branch from 4cd1bd9 to 7cb8fcd Compare July 15, 2019 07:29

bgirard force-pushed the master branch from 7cb8fcd to daca0b8 Compare July 15, 2019 08:05

sebmarkbage reviewed Jul 15, 2019

View reviewed changes

bgirard force-pushed the master branch from daca0b8 to 5ffe25f Compare July 17, 2019 04:39

bvaughn reviewed Jul 17, 2019

View reviewed changes

packages/react-reconciler/src/ReactFiberCompleteWork.js Show resolved Hide resolved

bgirard force-pushed the master branch from 5ffe25f to e244911 Compare July 17, 2019 16:47

sebmarkbage reviewed Jul 17, 2019

View reviewed changes

packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js Outdated Show resolved Hide resolved

sebmarkbage reviewed Jul 17, 2019

View reviewed changes

packages/react-reconciler/src/ReactFiberCommitWork.js Outdated Show resolved Hide resolved

bgirard force-pushed the master branch 2 times, most recently from 70ae6fc to 74898f6 Compare July 18, 2019 02:21

bgirard force-pushed the master branch from 74898f6 to 6c45a27 Compare July 19, 2019 23:24

sebmarkbage approved these changes Jul 23, 2019

View reviewed changes

packages/react-reconciler/src/ReactFiberCompleteWork.js Show resolved Hide resolved

bgirard force-pushed the master branch from 6c45a27 to 448fa2c Compare July 23, 2019 19:39

sebmarkbage merged commit 42b75ab into facebook:master Jul 24, 2019

Conversation

bgirard commented Jul 15, 2019

Uh oh!

sizebot commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

react-art

react-dom

react-test-renderer

react-reconciler

react-native-renderer

Uh oh!

sebmarkbage Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

sebmarkbage commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bgirard commented Jul 17, 2019

Uh oh!

Uh oh!

Uh oh!

sebmarkbage commented Jul 17, 2019

Uh oh!

sebmarkbage commented Jul 17, 2019

Uh oh!

Uh oh!

bgirard commented Jul 18, 2019

Uh oh!

bgirard commented Jul 19, 2019

Uh oh!

Uh oh!

bgirard commented Jul 23, 2019

Uh oh!

FokkeZB commented Dec 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sizebot commented Jul 15, 2019 •

edited

Loading

sebmarkbage commented Jul 16, 2019 •

edited

Loading